Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix some bug risks and code quality issues #115

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix some bug risks and code quality issues #115

wants to merge 1 commit into from

Conversation

sanketsaurav
Copy link

Changes:

  • In hammer/library/aws/s3.py, use in operator to check if variable is equal to either of two values.
  • In hammer/tools/ddb_inject_credentials.py, use is to compare with None.
  • In hammer/reporting-remediation/bot/commands.py, remove unnecessary else used after raise.
  • In hammer/library/aws/s3.py and hammer/library/ddb_issues.py, fix indentation not multiple of four.

This PR also adds .deepsource.toml configuration file to run Continuous Quality analysis on the repo with DeepSource. Upon enabling DeepSource, quality and security analysis will be run on every PR to detect 500+ types of problems in the changes — including bug risks, anti-patterns, security vulnerabilities, etc.

DeepSource is a free to use for open-source projects, and is used by teams at NASA, Uber, Slack among many others.

To enable DeepSource analysis after merging this PR, please follow these steps:

  1. Sign up on DeepSource with your GitHub account and grant access to this repo.
  2. Activate analysis on this repo here.

You can also look at the docs for more details. Do let me know if I can be of any help!

Cheers,

Sanket
Founder, DeepSource

@sanketsaurav
Fix some bug risks and code quality issues
55553de 
Changes:
- In `hammer/library/aws/s3.py`, use `in` operator to check if variable is equal to either of two values.
- In `hammer/tools/ddb_inject_credentials.py`, use `is` to compare with `None`.
- In `hammer/reporting-remediation/bot/commands.py`, remove unnecessary `else` used after `raise`.
- In `hammer/library/aws/s3.py` and `hammer/library/ddb_issues.py`, fix indentation not multiple of four.

Also added a `.deepsource.toml` configuration file to run continuous static analysis on the repository with DeepSource.
@sanketsaurav
Copy link
Author

You can look at all the existing issues on my fork of this repo here: https://deepsource.io/gh/sanketsaurav/hammer/issues/

@pranav1688
Copy link
Contributor

Thanks @sanketsaurav ! How would Deepsource compare against the likes of Semmle? I reviewed the issues pointed out here: https://deepsource.io/gh/sanketsaurav/hammer/issues/ , Would it be possible to get actionable solutions which can help resolve the issues.

Example: Invalid escape sequence found| FLK-W605
MAJOR BUG RISK
As of Python 3.6, a backslash-character pair that is not a valid escape sequence now generates a DeprecationWarning. Although this will eventually become a SyntaxError, that will not be for several Python releases.

hammer/identification/lambdas/api/authorizer.py
invalid escape sequence ‘*‘


 62    # The policy version used for the evaluation. This should always be '2012-10-17'
 63    version = '2012-10-17'
 64    # The regular expression used to validate resource paths for the policy
 65    pathRegex = '^[/.a-zA-Z0-9-\*]+$'

@sanketsaurav
Copy link
Author

Hey @pranav1688 — thanks for looking into this. Here are the differences, among many more, from other tools out there:

  • The results are optimized for fewer false-positives. DeepSource filters out noise intelligently by looking at the context. e.g. if you've defined your test_patterns, you won't see issues that don't make sense in test files.
  • It is easy to ignore issues for specific file patterns, test files, or the entire repo directly from the UI.
  • On pull requests, the issues detected are granular and only shown for the changed lines (rather than all lines in the files touched, like other tools).
  • You can track metrics like test coverage, documentation coverage, etc. all at one place with almost zero config.

About more actionable results: we currently have helpful descriptions for each issue raised. The ability to auto-fix issues is coming soon — so a lot of these trivial issues can be resolved with just a click. I'll keep you posted when we release it.

Would be happy to show you a demo and onboard the repo on DeepSource. Do let me know!

@pranav1688
Copy link
Contributor

Thanks for the information @sanketsaurav , I will be keen to know the auto-remediation when its ready.

@sanketsaurav
Copy link
Author

Sure @pranav1688! I'll keep you posted. You can start using DeepSource right away, though. I've added the configuration file for this repository in the PR. Do let me know if there's anything else that I can do to get you started. :)

@sanketsaurav
Copy link
Author

Hey @pranav1688 — just wanted to check if we can get this merged. Would love to see the project use DeepSource. Please let me know! :)

@mohi7solanki
Copy link

Hey @pranav1688 Mohit from DeepSource here, We have autofix / auto-remediation in production now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sanketsaurav @pranav1688 @mohi7solanki